Make ByteSourceJsonBootstrapper use StringReader for < 8KiB byte[] inputs
#1081
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 10, 2023
pjfanning
reviewed
Aug 10, 2023
|
|
||
| if (in == null) { | ||
| int length = _inputEnd - _inputPtr; | ||
| if (length >= 0 && length <= 8192) { |
There was a problem hiding this comment.
minor nit: It may not be worth checking length >= 0. It has a small perf impact and the length is very unlikely to negative and will just fail at the ByteArrayInputStream stage anyway. The length == 0 case will probably be more tidily handled with the StringReader than the ByteArrayInputStream route.
There was a problem hiding this comment.
I flipped the condition to short circuit for large inputs first if (length <= STRING_READER_BYTE_ARRAY_LENGTH_LIMIT && length >= 0).
I'd prefer to keep the length >= 0 as the array length check here should be negligible, and ensures the ByteInputStream continues handling any negative array length path.
There was a problem hiding this comment.
After thinking it through over night, I removed the length >= 0 condition.
cowtowncoder
changed the title
ByteSourceJsonBootstrapper use StringReader for < 8KiB byte[] inputs
|
Ok I assume this has been benchmarked and since it only affects what I assume is minority use case (non-UTF-8 input that comes as byte input), I am ok merging it. Thank you @schlosna ! |
cowtowncoder
added a commit
that referenced
this pull request
Aug 18, 2023
schlosna
added a commit
to palantir/atlasdb
that referenced
this pull request
Sep 22, 2023
Optimize to avoid allocation of heap ByteBuffer via InputStreamReader. Remove after upgrade to Jackson 2.16. see: FasterXML/jackson-core#1081 and FasterXML/jackson-benchmarks#9
bulldozer-bot bot
pushed a commit
to palantir/atlasdb
that referenced
this pull request
Sep 25, 2023
Optimize to avoid allocation of heap ByteBuffer via InputStreamReader. Remove after upgrade to Jackson 2.16. see: FasterXML/jackson-core#1081 and FasterXML/jackson-benchmarks#9
schlosna
added a commit
to palantir/atlasdb
that referenced
this pull request
Jun 27, 2024
Now that AtlasDB has upgraded to Jackson 2.16.1, remove performance workaround that landed upstream in Jackson 2.16.0. See FasterXML/jackson-core#1081
bulldozer-bot bot
pushed a commit
to palantir/atlasdb
that referenced
this pull request
Jul 1, 2024
Now that AtlasDB has upgraded to Jackson 2.16.1, remove performance workaround that landed upstream in Jackson 2.16.0. See FasterXML/jackson-core#1081 Removes changes from #6750
wendigo
added a commit
to trinodb/trino
that referenced
this pull request
Aug 5, 2024
Since FasterXML/jackson-core#1081 this optimization does no longer make sense as it's applied internally by Jackson if needed.
wendigo
added a commit
to trinodb/trino
that referenced
this pull request
Aug 6, 2024
Since FasterXML/jackson-core#1081 this optimization does no longer make sense as it's applied internally by Jackson if needed.
schlosna
added a commit
to palantir/conjure-java-runtime
that referenced
this pull request
May 12, 2025
Avoid InputStreamReader / HeapByteBuffer overhead for small (less than 8KiB) inputs, see FasterXML/jackson-core#1081
schlosna
added a commit
to palantir/conjure-java-runtime
that referenced
this pull request
Aug 12, 2025
Avoid InputStreamReader / HeapByteBuffer overhead for small (less than 8KiB) inputs, see FasterXML/jackson-core#1081
schlosna
added a commit
to palantir/conjure-java-runtime
that referenced
this pull request
Aug 20, 2025
Avoid InputStreamReader / HeapByteBuffer overhead for small (less than 8KiB) inputs, see FasterXML/jackson-core#1081
bulldozer-bot bot
pushed a commit
to palantir/conjure-java-runtime
that referenced
this pull request
Aug 21, 2025
Avoid `InputStreamReader` / `HeapByteBuffer` overhead for small (less than 8KiB) inputs, see FasterXML/jackson-core#1081 and FasterXML/jackson-benchmarks#9 (comment) for benchmarks showing between 2x and 10x speedup handling deserialization of small values.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From @carterkozak #1079 (comment) this is an alternative implementation draft to address #593 (and #995 (comment) ) when deserializing from a
byte[]as theInputStreamReadercode path triggers an 8KiBHeapByteBufferallocation forStreamDecoderregardless of input byte array length. This allocation significantly penalizes smallerbyte[]sources.The approach here converts
byte[]inputs smaller thank 8KiB to aStringand processed viaStringReader. This should avoid unnecessary 8KiB heap byte buffer allocation and leverage OpenJDK's continued charset decoding improvements (e.g. https://cl4es.github.io/2021/02/23/Faster-Charset-Decoding.html ).Initial benchmarks from FasterXML/jackson-benchmarks#9 show
StringReaderproviding performance equivalent toByteArrayInputStreamsource in worst case, and anywhere from ~2x to ~10x speedup in best case.